Skip to content

Branch-reconcile finishes a review branch whose run died before opening a PR - #14449

Merged
atomantic merged 1 commit into
mainfrom
fix/reconcile-unshipped-review-claim
Oct 31, 2026
Merged

atomantic merged 1 commit into
mainfrom
fix/reconcile-unshipped-review-claim

Conversation

@atomantic

Copy link
Copy Markdown
Owner

Problem

A claim-issue run with review-then-merge completion (claim/issue-14241) was cut off after committing and before pushing. It left a clean local branch with 2 commits, no remote ref, and no PR. Every branch-reconcile cycle parked with "no coordinator task eligible" and never mentioned the branch:

  • readBranchCompletionHold returned pr-completion-review-required.
  • The classifier's completion-policy rule, which runs second, made the branch WIP, so it never reached NEEDS_PR.
  • The retirement pass only looks for a merged PR to retire against, and there was none.

The branch was shipped by hand in #14447.

Fix

The review obligation gates the merge, not the work. readBranchCompletionHoldDetail now also reports unshipped: a review-then-merge obligation with no recorded PR, no open PR, and no leave-open handoff. When the commits are also not on the default branch, the completion-policy rule lets the branch fall through to the ordinary rules. A clean branch with commits becomes NEEDS_PR, and the coordinator opens its PR through /do:pr --no-merge and the reviewer loop.

These cases are unchanged:

  • A merged pointer, an open PR, or a recorded PR stays under the completion-policy hold (the retirement proof is still required).
  • A live owner and a leave-open human handoff keep their holds.
  • The merge gate (cosMergeHandoff) still enforces the review policy once the PR exists.

Tests

  • branchCompletionPolicy.test.js: through gatherBranchState and classifyBranches, an unshipped review branch is NEEDS_PR, and it goes back under the completion-policy hold once a PR is recorded.
  • branchReconcile.test.js: the rule matrix covers unshipped → NEEDS_PR, plus merged, open-PR, and live-owner staying held.
  • Mutation probe: reverting the classifier change fails both new tests.
  • The 7 branch-reconcile and pre-step suites pass (436 tests), and npm run pregate passes.

…opening a PR

A claim run configured for review-then-merge that was cut off after committing
but before pushing left a local branch with real commits, no PR, and a
`pr-completion-review-required` hold. The classifier's completion-policy rule
turned that hold into WIP, so the branch never reached the coordinator, and the
retirement pass found no merged PR to retire it against. The work sat stranded
on every reconcile cycle.

The review obligation gates the merge, not the work. When no PR was ever
recorded for it, none is open, and the commits are not on the default branch,
the hold now falls through to the ordinary rules: a clean branch becomes
NEEDS_PR and the coordinator opens its PR through `/do:pr --no-merge` and the
reviewer loop. A merged pointer, an open or recorded PR, a leave-open handoff
and a live owner keep their holds unchanged, and the merge gate still enforces
the review policy.
@atomantic atomantic added bug Proposed from a runtime-safety audit implementer:opus-5-5 PR opened by the opus-5-5 model labels Oct 31, 2026
@atomantic
atomantic enabled auto-merge (rebase) October 31, 2026 15:24
@atomantic
atomantic merged commit 31f0f75 into main Oct 31, 2026
16 checks passed
@atomantic
atomantic deleted the fix/reconcile-unshipped-review-claim branch October 31, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Proposed from a runtime-safety audit implementer:opus-5-5 PR opened by the opus-5-5 model

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant